-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move error::Error from libstd to liballoc #64017
Conversation
@rust-lang/libs, any thoughts on this? |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
It's there any reason not to do this? It otherwise seems like a big win. (And ideally, Error would be in core too, although it sounds like the downcast API makes that tricky I guess?) |
There’s a code comment about impl coherence not allowing the trait to be in libcore: Lines 5 to 14 in 19a38de
Having it in liballoc sounds ok to me. Maybe even doing FCP to stabilize the new location in this PR, if other team members are ok with that. |
This reverts commit 47c3ad1.
(I think it would be preferable if it could be stabilized in this PR, so I reverted the commit on unstable attributes.) |
RFC 2504, which hasn't been fully implemented, intends to expose a |
Thank you @taiki-e for the PR. I will close it now as it unfortunately appears to be incompatible with RFC 2504 which is already accepted (even if not implemented yet). I’m not sure whether backtrace functionality qualifies for libcore/liballoc (that is, can it always be available everywhere Rust can run, without a dependency on the environment or operating system). But even if it does, the RFC currently specifies a behavior that relies on an environment variable and Maybe we could decide to have a |
Hard to remember 2 years later, but I believe this is what I did to make However, I think the best solution is to move to a feature-based architecture so that traits in core can have methods added in std, because core/alloc/std are all the same crate under different feature flags. But that's a big project. |
This comment has been minimized.
This comment has been minimized.
To clarify: I did not make a judgment of what is preferable, nor did I say that Error in liballoc could never happen. Only that since we already accepted an RFC for
I did propose a way above. I feel that if you’re writing 1600+ words for everyone else to read, the least you can do is read the comments you’re responding to.
That would have been relevant discussion when this impl was being proposed. Now that’s it’s stable we can’t simply remove it.
It’s not. This signature has no bound on the type Result<T> = std::result::Result<T, Box<dyn Any + Send + 'static>>;
pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> {…} |
@cbeck88 I really encourage you to read in detail RFC 2504 as well the discussion on the pull request - rust-lang/rfcs#2504. Your comment evidences a confusion about some of the issues at play, and I have to be honest that I find it rather impolitely bombastic for not understanding the situation clearly. There are two viable paths toward making Error available to no_std crates:
|
This moves
Error
trait fromlibstd
toliballoc
.When importingError
trait fromliballoc
, it is necessary to enable an unstable feature (As I referred to #51846, I’m making this unstable, but is it actually unnecessary?).Unlike #51846, this is just moving the trait, so I don't think RFC is needed, but I'm not sure how this will affect #53487 (backtrace). cc @alexcrichton
Closes #62502
r? @SimonSapin